Mysqli: fix missing error for invalid mysqli_options() option #20971

arshidkv12 · 2026-01-19T03:13:43Z

#20968

…cting report_mode

iluuu1994

From what I can see (as somebody who doesn't maintain this code), most arguments are validated without the MYSQLI_REPORT_ERROR guard, and throw a zend_argument_value_error().

But somebody who does maintain this should confirm.

ext/mysqli/mysqli_api.c

ext/mysqli/tests/gh20968.phpt

ext/mysqli/mysqli_api.c

Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>

…cting report_mode

arshidkv12 · 2026-01-19T16:18:24Z

From what I can see (as somebody who doesn't maintain this code), most arguments are validated without the MYSQLI_REPORT_ERROR guard, and throw a zend_argument_value_error().

But somebody who does maintain this should confirm.

- if (MyG(report_mode) & MYSQLI_REPORT_ERROR) {
	zend_value_error("mysqli_options(): Invalid option %d", (int)mysql_option);

Do we need to remove if (MyG(report_mode) & MYSQLI_REPORT_ERROR)? What do you think?

iluuu1994 · 2026-01-19T17:07:10Z

Somebody else should review this. /cc @Girgias @SakiTakamachi

kamil-tekiela · 2026-01-19T17:28:21Z

First of all, I don't know if this should be an error condition. I guess it kind of makes sense. But as you can see in the test, no message was the behaviour by design.

The error should be without the guard and without the function name in the message. It seems to me like you have also put it in the wrong place. Judging by the message and the test, you wanted to put it in the switch statement after that.

The message isn't very descriptive either. What does it mean "invalid option"? What should the user do differently?

…cting report_mode

arshidkv12 · 2026-01-20T05:45:34Z

Changed the error message to zend_value_error("Argument #1 ($option) is not a valid mysqli option");

ext/mysqli/mysqli_api.c

ext/mysqli/tests/mysqli_options.phpt

ext/mysqli/tests/mysqli_set_opt.phpt

Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>

…cting report_mode

Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>

arshidkv12 · 2026-01-20T13:35:14Z

Thank you @kamil-tekiela kamil-tekiela

kamil-tekiela · 2026-01-20T13:37:06Z

@iluuu1994 Are you ok with how this looks now?

…cting report_mode

iluuu1994

I'm not a codeowner, but yes, LGTM now. Thanks @arshidkv12!

ext/mysqli/tests/mysqli_set_opt.phpt

…cting report_mode

Girgias · 2026-01-20T17:54:26Z

ext/mysqli/tests/gh20968.phpt

@@ -0,0 +1,27 @@
+--TEST--
+Bug #20968 mysqli_options() with invalid option triggers ValueError


Suggested change

Bug #20968 mysqli_options() with invalid option triggers ValueError

GH-20968 mysqli_options() with invalid option triggers ValueError

We use Bug #xxxxx notation to refer to old bugsnet bugs.

Girgias · 2026-01-20T17:54:59Z

ext/mysqli/tests/gh20968.phpt

+
+$mysqli = new mysqli($host, $user, $passwd, $db, $port, $socket);
+
+$value = $mysqli->options(10, 'invalid_option');


Add a try catch here and check the exception message, we don't care about the stack trace.

Girgias · 2026-01-20T17:56:02Z

ext/mysqli/mysqli_api.c

 	expected_type = mysqli_options_get_option_zval_type(mysql_option);
+	if (expected_type == IS_NULL) {
+		zend_argument_value_error(ERROR_ARG_POS(2), "must be one of predefined options");
+		RETURN_THROWS();
+	}


We normally list the available options, and if there are too many we at least provide a prefix for the error eg. MYSQLI_BLAH_X

I thought about that too, but there isn't any common pattern between them. One of the values doesn't even have a corresponding constants.

That seems kinda bad that one of the options doesn't have a constant?

As future scope, would it make sense to have an Enum of MySQLi options? As that would make the type checking and value validation easy.

Yes, it is. Maybe we should add the constant. Enums would definitely be better but it would be a breaking change difficult for cross-version compatibilty.

Yes, it is. Maybe we should add the constant. Enums would definitely be better but it would be a breaking change difficult for cross-version compatibilty.

It would be possible to assign the enum case as the constant value, or support int|Enum. But I think already adding the missing constant is the big thing.

Girgias · 2026-01-20T17:56:47Z

ext/mysqli/tests/gh20968.phpt

@@ -0,0 +1,27 @@
+--TEST--
+Bug #20968 mysqli_options() with invalid option triggers ValueError


Suggested change

Bug #20968 mysqli_options() with invalid option triggers ValueError

Bug #20968 mysqli_options() with invalid option should triggers ValueError

The phrasing of the bug title seems to imply that an invalid option MUST NOT thow a ValueError, which is the complete opposite of what is being done.

…cting report_mode

arshidkv12 added 2 commits January 19, 2026 08:35

Fix bug gh20968

a401699

mysqli: raise ValueError for invalid option in mysqli_options() respe…

217b19b

…cting report_mode

arshidkv12 requested review from bukka and kamil-tekiela as code owners January 19, 2026 03:13

github-actions bot added the Extension: mysqli label Jan 19, 2026

arshidkv12 added 5 commits January 19, 2026 08:52

mysqli: raise ValueError for invalid option in mysqli_options() respe…

66fbc0a

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

54a8e06

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

0716f97

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

d7dd938

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

1aa360a

…cting report_mode

iluuu1994 reviewed Jan 19, 2026

View reviewed changes

ext/mysqli/mysqli_api.c Outdated Show resolved Hide resolved

ext/mysqli/tests/gh20968.phpt Outdated Show resolved Hide resolved

ext/mysqli/mysqli_api.c Outdated Show resolved Hide resolved

arshidkv12 and others added 3 commits January 19, 2026 21:33

Update ext/mysqli/mysqli_api.c

04f8006

Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>

Update ext/mysqli/tests/gh20968.phpt

9ba4bf2

Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>

mysqli: raise ValueError for invalid option in mysqli_options() respe…

2db6e98

…cting report_mode

arshidkv12 requested a review from iluuu1994 January 19, 2026 16:50

arshidkv12 added 3 commits January 20, 2026 11:09

mysqli: raise ValueError for invalid option in mysqli_options() respe…

29ed278

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

1647ae8

…cting report_mode

mysqli: raise ValueError for invalid option in mysqli_options() respe…

0a98c9c

…cting report_mode

kamil-tekiela reviewed Jan 20, 2026

View reviewed changes

ext/mysqli/mysqli_api.c Outdated Show resolved Hide resolved

kamil-tekiela reviewed Jan 20, 2026

View reviewed changes

ext/mysqli/tests/mysqli_options.phpt Outdated Show resolved Hide resolved

kamil-tekiela reviewed Jan 20, 2026

View reviewed changes

ext/mysqli/tests/mysqli_set_opt.phpt Outdated Show resolved Hide resolved

arshidkv12 and others added 4 commits January 20, 2026 18:59

Update ext/mysqli/mysqli_api.c

8193542

Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>

Update ext/mysqli/tests/mysqli_options.phpt

3a633b3

Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>

mysqli: raise ValueError for invalid option in mysqli_options() respe…

072d1a4

…cting report_mode

Update ext/mysqli/tests/mysqli_set_opt.phpt

a188868

Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>

mysqli: raise ValueError for invalid option in mysqli_options() respe…

e102967

…cting report_mode

iluuu1994 approved these changes Jan 20, 2026

View reviewed changes

ext/mysqli/tests/mysqli_set_opt.phpt Outdated Show resolved Hide resolved

kamil-tekiela approved these changes Jan 20, 2026

View reviewed changes

mysqli: raise ValueError for invalid option in mysqli_options() respe…

6a71d4f

…cting report_mode

Girgias requested changes Jan 20, 2026

View reviewed changes

kamil-tekiela mentioned this pull request Jan 20, 2026

Add MYSQLI_OPT_COMPRESS #20987

Merged

mysqli: raise ValueError for invalid option in mysqli_options() respe…

d1df211

…cting report_mode

		@@ -0,0 +1,27 @@
		--TEST--
		Bug #20968 mysqli_options() with invalid option triggers ValueError

	Bug #20968 mysqli_options() with invalid option triggers ValueError
	GH-20968 mysqli_options() with invalid option triggers ValueError


		$mysqli = new mysqli($host, $user, $passwd, $db, $port, $socket);

		$value = $mysqli->options(10, 'invalid_option');

Mysqli: fix missing error for invalid mysqli_options() option #20971

Are you sure you want to change the base?

Mysqli: fix missing error for invalid mysqli_options() option #20971

Conversation

arshidkv12 commented Jan 19, 2026

Uh oh!

iluuu1994 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arshidkv12 commented Jan 19, 2026

Uh oh!

iluuu1994 commented Jan 19, 2026

Uh oh!

kamil-tekiela commented Jan 19, 2026

Uh oh!

arshidkv12 commented Jan 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arshidkv12 commented Jan 20, 2026

Uh oh!

kamil-tekiela commented Jan 20, 2026

Uh oh!

iluuu1994 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants